Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CI: modify to reusable workflow #482

Closed
wants to merge 42 commits into from
Closed

CI: modify to reusable workflow #482

wants to merge 42 commits into from

Conversation

Dominik-K
Copy link
Contributor

@Dominik-K Dominik-K self-assigned this Dec 19, 2023
Signed-off-by: Dominik <[email protected]>
@Dominik-K Dominik-K changed the title CI: change to reusable workflow CI: modify to reusable workflow Dec 19, 2023
@Dominik-K Dominik-K force-pushed the CI/reusable-workflow branch from 9b9850f to 1b93218 Compare December 21, 2023 09:51
Signed-off-by: Dominik <[email protected]>
docker commit test-container integration-image
pushd source/.ci/e2e
docker-compose run e2e-test-server run-script tests
docker commit compile-container integration-tests-image
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do the integration-tests rely on the unit-tests somehow?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think so :)

@Dominik-K Dominik-K force-pushed the CI/reusable-workflow branch from 56f22bf to 5928cd8 Compare December 23, 2023 00:51
@Dominik-K Dominik-K force-pushed the CI/reusable-workflow branch from 7efd158 to 6d4a2e1 Compare December 23, 2023 01:19
@Dominik-K Dominik-K force-pushed the CI/reusable-workflow branch from 1a17cd2 to 2147cb8 Compare December 23, 2023 01:38
Signed-off-by: Dominik <[email protected]>
Signed-off-by: Dominik <[email protected]>
Signed-off-by: Dominik <[email protected]>
@Dominik-K Dominik-K marked this pull request as ready for review January 10, 2024 15:15
@Dominik-K Dominik-K requested review from a-w50 and andistorm January 10, 2024 15:15
uses: everest/everest-ci/github-actions/[email protected]
path: ${{ env.CI_REPO_DIR }}
repository: ${{ env.CI_REPO }}
ref: CI/reusable-workflow # TODO get reference in general, see https://stackoverflow.com/questions/74784735 resp. https://github.com/actions/toolkit/issues/1264
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@andistorm Do you know a solution for this? Otherwise, I would just set it to main for now.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would suggest to not checkout the ci repo in the workflow, but to use custom github actions. There are multiple options for github actions: composite actions that are similiar to reusable workflows, Javascript actions and Docker container actions. The last one would be an option to include additional filesto the action.

I could also imagine a solution with having the bash scripts in each repo with "custom" commands and cmake flags..

@Dominik-K Dominik-K requested a review from hikinggrass January 10, 2024 15:56
@hikinggrass hikinggrass marked this pull request as draft January 25, 2024 14:58
mkdir scripts
rsync -a source/.ci/build-kit/ scripts
path: ${{ env.SOURCE_DIR }}
repository: ${{ inputs.repository }}

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This doesn't seem like the right way to set up a reusable workflow. When someone makes a fork, the CI instructions on their forked branches will point to whatever original repository is passed in, for example, EVerest/libocpp. What should happen is that the current branch that initialized the workflow should run, fetching the branch from the forked repository.

In the current state, this fails at the checkout stage because we can't check out the original repository from our fork.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't see the problem in the way the reusable workflow is implemented, but the way it is called.

In the example of libocpp the parameter repository: EVerest/libocpp is used. You adapted it similiar in your example with repository: EVerest/libfsm

To enable workflows running on forks I suggest the use of the env variables
.

-> In both examples I would replace the line with repository: ${{ github.repository }}.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't the reusable workflow use that by default?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah, maybe this parameter is obsolete, not sure about this.. I will spend some time on this in the next weeks to figure those things out

@andistorm andistorm added the CI/CD label Feb 5, 2024
@golovasteek
Copy link
Contributor

The everest-core is an umbrella-repository that depends on all other EVerest components. If the reusable workflow will be part of everest-core, then all components will depend on the everest-core.
This looks like a cyclic dependency. Do we see a problem with it? Did we consider other places to "host" the reusable workflow?

@andistorm
Copy link
Contributor

The everest-core is an umbrella-repository that depends on all other EVerest components. If the reusable workflow will be part of everest-core, then all components will depend on the everest-core. This looks like a cyclic dependency. Do we see a problem with it? Did we consider other places to "host" the reusable workflow?

Thank you for the notice. I planned to shift the workflow to everest-ci.

@andistorm andistorm assigned andistorm and unassigned Dominik-K Apr 29, 2024
@andistorm
Copy link
Contributor

Close in favor of upcomming reusable workflow in everest-ci EVerest/everest-ci#54

@andistorm andistorm closed this Oct 7, 2024
@andistorm andistorm deleted the CI/reusable-workflow branch October 7, 2024 07:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants